Ignore missing URL parts in NavItem activity check #172
Conversation
src/django_simple_nav/nav.py
Outdated
| parsed_url.scheme | ||
| and (parsed_url.scheme != parsed_request.scheme) | ||
| or parsed_url.netloc | ||
| and (parsed_url.netloc != parsed_request.netloc) |
There was a problem hiding this comment.
Please group these by the order of operations so that it's easier to maintainer.
I think you were going for roughly:
if (
(parsed_url.scheme and parsed_url.scheme != parsed_request.scheme)
or
(parsed_url.netloc and parsed_url.netloc != parsed_request.netloc)
):There was a problem hiding this comment.
Boolean operators have higher precedence than comparison operators, so wouldn't (parsed_url.scheme and parsed_url.scheme != parsed_request.scheme) be the same as ((parsed_url.scheme and parsed_url.scheme) != parsed_request.scheme)? This would compare a boolean to a string, which will always fail. What I wanted to express was (parsed_url.scheme and (parsed_url.scheme != parsed_request.scheme)).
There was a problem hiding this comment.
That makes sense. I think the latter might make the most sense then. I think the grouping would spell it out more and be easier for us to maintain.
There was a problem hiding this comment.
OK, I have added parentheses around the operators of the or.
|
bump Is there anything that's missing? |
joshuadavidthomas
left a comment
joshuadavidthomas
left a comment
There was a problem hiding this comment.
Yeah, looks good. Sorry for the delay in actually reviewing this, thanks for the contribution.
Closes westerveltco#169. If the URL of a navigation item is missing the scheme or the network location we ignore it in the check.
for more information, see https://pre-commit.ci
joshuadavidthomas
force-pushed
the
get-active
branch
from
bc78bb2 to
a8bf324
Compare
3 participants
Closes #169. If the URL of a navigation item is missing the scheme or the network location we ignore it in the check.